Conversation
ncloudioj
approved these changes
Jun 25, 2025
Member
ncloudioj
left a comment
ncloudioj
left a comment
There was a problem hiding this comment.
This looks great, thanks!
Merino could take lots of inspirations from here to handle i18n in the future. 👍
0c0w3
force-pushed
the
adw/suggest-weather-keywords-collate-bug-1973876
branch
2 times, most recently
from
8ee6255 to
8f0b430
Compare
This creates a new `keywords_i18n` table that's the same as `keywords` except the main column uses the collation from the geonames table. Weather keywords are now added to this table instead of the old one. I think we should use the new table in the future especially if/as we expand to non-English locales, but it's useful for English too since it ignores some punctuation. Since the collation is now used for other things besides geonames, I renamed it `i18n_collation`. Let me know if you have ideas on better names for the table and collation. I used "i18n" because I wanted to capture the idea they're useful for locale-specific user-facing strings. "i18n" seemed more appropriate than "l10n" because these things are structures/facilities that enable l10n and aren't the l10n artifacts themselves. I thought about using "unicode" but that's pretty ambiguous I think. This also makes some improvements to keywords metrics.
0c0w3
force-pushed
the
adw/suggest-weather-keywords-collate-bug-1973876
branch
from
8f0b430 to
fbf31a2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This creates a new
keywords_i18ntable that's the same askeywordsexcept the main column uses the collation from the geonames table. Weather keywords are now added to this table instead of the old one.I think we should use the new table in the future especially if/as we expand to non-English locales, but it's useful for English too since it ignores some punctuation.
Since the collation is now used for other things besides geonames, I renamed it
i18n_collation.Let me know if you have ideas on better names for the table and collation. I used "i18n" because I wanted to capture the idea they're useful for locale-specific user-facing strings. "i18n" seemed more appropriate than "l10n" because these things are structures/facilities that enable l10n and aren't the l10n artifacts themselves. I thought about using "unicode" but that's pretty ambiguous I think.
This also makes some improvements to keywords metrics.
Pull Request checklist
[ci full]to the PR title.